Skip to content

Maintain reference to root type through use of handler as $rootType#36

Open
cdbattags wants to merge 3 commits intobsalex:masterfrom
cdbattags:master
Open

Maintain reference to root type through use of handler as $rootType#36
cdbattags wants to merge 3 commits intobsalex:masterfrom
cdbattags:master

Conversation

@cdbattags
Copy link

Reference to #35

@cdbattags
Copy link
Author

cdbattags commented Sep 9, 2023

Pretty happy with the way this turned out. I think one enhancement would be to add replace default typing like so:

export function typedPath<
    RootObjectType,
    HandlersType extends TypedPathHandlersConfig = Record<never, never>,
    OriginalObjectType =  RootObjectType,
>(
    additionalHandlers?: HandlersType,
    path: TypedPathKey[] = []
): TypedPathWrapper<RootObjectType, HandlersType & DefaultHandlers<RootObjectType>, OriginalObjectType> {}

This way the tests should be able to work as is and would add backwards compatibility for everyone.

But even then, it'd be a little tricky. I'll leave the rest to you and continue to use this in our application as-is until we can discuss!

@bsalex
Copy link
Owner

bsalex commented Sep 10, 2023

Thank you for your contribution!
I agree with your concerns about backward compatibility.
Let's investigate this issue.

@bsalex
Copy link
Owner

bsalex commented Sep 10, 2023

Hey @cdbattags
Could you please check #37 ?
The idea there is the same as in your PR, but the $rootType moved outside to additional handlers.
This way we can preserve the backward compatibility and keep the static internal default handlers.
With static internal default handlers, we can save some resources on the function calls and same object initialization (https://github.com/bsalex/typed-path/pull/36/files#diff-dcdc3e0b3362edb8fec2a51d3fa51f8fb8af8f70247e06d9887fa934834c9122R89).

@cdbattags
Copy link
Author

cdbattags commented Sep 11, 2023

So I do like the concept of the additional handler but it then requires me to jump through a lot more hoops in my application code to match the "$rootType" how I want to.

I use it like so:

type CustomTypedPathWrapper<T> = TypedPathWrapper<T, unknown, DefaultHandlers<T>>

export class Differences<T> {
  constructor(private changed: T, public raw: Array<Difference>) {}

  filter(pathFilter: CustomTypedPathWrapper<T>) {
    return this.raw.filter(({ path, value }) => matchPath(path, typedPathToRegex(pathFilter)))
  }
}

How would I be able to set the pathFilter argument for filter function with the additional handler method you have in #37?

@cdbattags
Copy link
Author

cdbattags commented Sep 11, 2023

Full usage with only handler way that shows how it doesn't really work:

import diff, { Difference, DifferenceChange } from 'microdiff'
import { typedPath as originalTypedPath } from 'typed-path'

import { Quote } from '@/entities/Quote'

function getAdditionalHandlers<RootType>() {
  return {
    $rootType: () => ({} as RootType),
  }
}

const typedPath = <T>() => originalTypedPath<T, ReturnType<typeof getAdditionalHandlers<T>>>(getAdditionalHandlers<T>())

type TypedPathWrapper<T> = ReturnType<typeof typedPath<T>>

const matchPath = (path: (string | number)[], property: RegExp) => {
  const joined = path.join(`.`)
  return property.test(joined)
}

const typedPathToRegex = <T>(tp: TypedPathWrapper<T>) => {
  const regex = new RegExp(tp.$rawPath.join(`.`).replace(/\d+/g, `\\d+`))
  return regex
}

export type { Difference }

type DifferenceTypes = Difference[`type`]

export class Differences<T> {
  constructor(private changed: T, public raw: Array<Difference>) {}

  filter({
    pathFilter,
    typeFilter,
    valueFilter,
  }: {
    pathFilter?: TypedPathWrapper<T>
    typeFilter?: (type: DifferenceTypes) => boolean
    valueFilter?: (value: any, oldValue?: any) => boolean
  }) {
    return this.raw.filter((d) => {
      let pathMatch = true
      let typeMatch = true
      let valueMatch = true

      if (pathFilter) {
        pathMatch = matchPath(d.path, typedPathToRegex(pathFilter))
      }

      if (typeFilter) {
        typeMatch = typeFilter(d.type)
      }

      if (valueFilter) {
        const { value, oldValue } = d as DifferenceChange
        valueMatch = valueFilter(value, oldValue)
      }

      return pathMatch && typeMatch && valueMatch
    })
  }
}

export const getDifferences = <T extends Record<string, any> | any[]>(left: T, right: T) =>
  new Differences<T>(left, diff(left, right))

const differences = new Differences(Quote, [])
differences.filter(typedPath<Quote>().loads[0].status)

How would I be able to successfully check that the pathFilter has the right derived type?

@cdbattags
Copy link
Author

cdbattags commented Sep 11, 2023

So more or less I can drop the $rootType completely from the handlers and only have it in the types for my usage.

And the only way to get past this error Type parameter defaults can only reference previously declared type parameters. is to have the type ordering done the way I proposed.

So maybe this is a v3 change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants